Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ppc64le core-bpf testing and PR testing #1448

Merged
merged 63 commits into from
Jan 3, 2024

Conversation

Stringy
Copy link
Collaborator

@Stringy Stringy commented Nov 29, 2023

Description

Should enable core-bpf testing for ppc64le. Once it is working/failing, I'll rebase it over the vanilla branch.

Checklist

  • Investigated and inspected CI test results
  • Updated documentation accordingly

Automated testing

  • Added unit tests
  • Added integration tests
  • Added regression tests

If any of these don't apply, please comment below.

Testing Performed

TODO(replace-me)
Use this space to explain how you tested your PR, or, if you didn't test it, why you did not do so. (Valid reasons include "CI is sufficient" or "No testable changes")
In addition to reviewing your code, reviewers must also review your testing instructions, and make sure they are sufficient.

For more details, ref the Confluence page about this section.

Copy link

openshift-ci bot commented Nov 29, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@Stringy Stringy added build-full-images run-multiarch-builds Run steps for non-x86 archs. labels Nov 29, 2023
@Stringy Stringy force-pushed the giles/ppc64le-core-bpf-testing branch from b093279 to 40d3d97 Compare November 30, 2023 13:19
@Stringy Stringy changed the base branch from master to mauro/use-falco-upstream November 30, 2023 13:19
@Stringy Stringy marked this pull request as ready for review November 30, 2023 13:20
@Stringy Stringy requested a review from a team as a code owner November 30, 2023 13:20
@Stringy Stringy force-pushed the giles/ppc64le-core-bpf-testing branch from 40d3d97 to e86fcce Compare December 1, 2023 13:31
@Stringy Stringy force-pushed the giles/ppc64le-core-bpf-testing branch from e86fcce to 622b6d6 Compare December 1, 2023 13:57
@Stringy Stringy force-pushed the giles/ppc64le-core-bpf-testing branch from bdfede0 to 81d8f68 Compare December 8, 2023 08:37
@Stringy Stringy added the integration-tests-trace-logging Turn on additional logging in Collector label Dec 12, 2023
@Stringy Stringy force-pushed the giles/ppc64le-core-bpf-testing branch 10 times, most recently from 4e05a88 to de0f77e Compare December 20, 2023 12:49
Copy link
Contributor

@erthalion erthalion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good. I've left few commentaries, but suggest to follow up on them in a separate PR. Otherwise not sure what's your plan to proceed -- s390x tests seems to pass, but few others are failing, is that expected?

@@ -11,11 +11,12 @@ on:
outputs:
collector-builder-tag:
description: The builder tag used by the build
value: ${{ jobs.build-builder-image.outputs.collector-builder-tag || 'master' }}
value: ${{ jobs.build-builder-image.outputs.collector-builder-tag || '3.16.x-195-g8f32e71fad' }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was this image exactly? And would it be dropped before merging?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was taken from mauro's original vanilla PR and will be dropped before merging.

@@ -110,6 +143,24 @@ jobs:
-e @'${{ github.workspace }}/ansible/secrets.yml' \
ansible/ci-build-builder.yml

- name: Build s390x images
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels a bit weird, is it easier to build s390x images in a separate step, not as part of the previous one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, yeah it's a little simpler, just because we can force the previous step to use localhost only, but this step will use the entire inventory (which is only one VM at the moment)

When I implement full multi-arch native builds I'll hopefully combine these steps

ANSIBLE_CONFIG: ansible/ansible.cfg
VM_TYPE: rhel-s390x

- name: Save CMake cache
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this cache be used automatically, or does it have to be explicitly included? It's probably a good idea to add a condition to allow running without cache if specified via a label.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just for debugging the build, I'll remove it.

@@ -19,7 +19,7 @@
- name: Build full image
when:
- build_full_image
- arch != 'arm64'
- arch != 'arm64' and arch != 'ppc64le' and arch != 's390x'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the part where we temporarily disable full builds for multi-arch, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it is. I'll remove before merging.

version: "{{ collector_git_sha }}"
refspec: "+{{ collector_git_ref | replace('refs/', '') }}"
recursive: true
when: arch == "s390x"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I follow, why to clone only for s390x?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it's the only platform (currently) that is building on a separate machine to the GHA runner. All the other platforms can build from the existing checked out source

@Stringy
Copy link
Collaborator Author

Stringy commented Jan 2, 2024

s390x tests seems to pass, but few others are failing, is that expected?

Seems to have been an SSH key mix up following my ansible changes; I've fixed that now so this run should pass 🤞

@Stringy Stringy force-pushed the giles/ppc64le-core-bpf-testing branch from 5528455 to dc521ae Compare January 3, 2024 08:54
@Stringy Stringy merged commit 16fa1e4 into mauro/use-falco-upstream Jan 3, 2024
84 of 85 checks passed
@Stringy Stringy deleted the giles/ppc64le-core-bpf-testing branch January 3, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-full-images integration-tests-trace-logging Turn on additional logging in Collector run-multiarch-builds Run steps for non-x86 archs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants